Skip to content

Search content/people/topic#14

Open
MaxPoon wants to merge 17 commits intolzjun567:masterfrom
MaxPoon:search
Open

Search content/people/topic#14
MaxPoon wants to merge 17 commits intolzjun567:masterfrom
MaxPoon:search

Conversation

@MaxPoon
Copy link
Copy Markdown
Contributor

@MaxPoon MaxPoon commented Apr 25, 2017

先发上来看看有什么建议

Todo:

  • 建立搜索人物、话题的类似function
  • 允许设定要求的搜索结果数量(目前默认为10条,与在知乎上使用搜索数量一样)
  • 创建单元测试

Comment thread zhihu/models/search.py Outdated

class Search(Model):
@need_login
def search_content(self, q=''):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. search_content 作为一个方法名太含糊了
  2. 参数名最好明确一点,不要叫 q ,没必要和知乎请求参数保持一致
  3. 没考虑分页,万一我传了个很简单的关键字,查出来一万条,你也直接返回么?或者,你知道那种情况下需要多长时间这个方法才能执行完么?
  4. 小伙子加油!整体还不错~

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

谢谢你的建议!

  1. 有什么好建议吗?search question?
  2. 其实发送一次request它是固定返回10条搜索结果的,所以不存在结果太多的问题。如果是需要更多结果可以继续发request,params中加个offset即可,每个request都是返回10条结果。现在问题在于这十条之中有可能存在专栏。在考虑怎样写比较优雅。

@MaxPoon
Copy link
Copy Markdown
Contributor Author

MaxPoon commented May 23, 2017

当初设想要实现的功能都实现了,帮忙看看能不能merge吧

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants